Change "127.0.0.1" to SIDECAR_IP to dynamically account for IPv4 vs. IPv6 environments by willtsai · Pull Request #649 · dapr/java-sdk

willtsai · 2021-11-04T06:09:44Z

Description

I've made some changes to move away from hardcoded "127.0.0.1" IP addresses (representing localhost) to make the SDK and Tests compatible with exclusively IPv6 environments:

Replaced hardcoded "127.0.0.1" in various URL strings with the SIDECAR_IP property constant
Changed the DEFAULT_SIDECAR_IP property constant's value from the hardcoded "127.0.0.1" to instead use the Java API getLoopbackAddress(). This would make the IP default value more dynamic and account for situations where users operate in an exclusively ipv6 environment and the logic ends up falling back to the default value.
Added a test util to detect IPv6 addresses and format them accordingly for HTTP URLs using the square brackets representation in unit tests

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #218

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

Code compiles correctly
Created/updated tests
Extended the documentation (N/A)

mukundansundar · 2021-11-12T17:14:40Z

@artursouza Wasn't there was a particular reason why 127.0.0.1 was used instead of localhost?

willtsai · 2021-11-12T18:22:14Z

@artursouza Wasn't there was a particular reason why 127.0.0.1 was used instead of localhost?

@mukundansundar according to #213, it was changed from localhost to 127.0.0.1 "to avoid local OS specific DNS lookups that might hinder performance". My changes here don't reintroduce the use of localhost, but rather uses the dynamically sourced SIDECAR_IP constant from the system.

willtsai · 2021-12-06T07:40:03Z

Work in progress, please do not merge

Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

* Upgrade okhttp-mock dependency Version 1.3.2 was never deployed to Maven Central * No longer refer to JCenter, as it has been shut down Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

Bumps [actions/setup-java](https://github.com/actions/setup-java) from 2.3.1 to 2.4.0. - [Release notes](https://github.com/actions/setup-java/releases) - [Commits](actions/setup-java@v2.3.1...v2.4.0) --- updated-dependencies: - dependency-name: actions/setup-java dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

* Upgrade Dapr CLI and runtime. Signed-off-by: Artur Souza <artursouza.ms@outlook.com> * Fix expected vault output. Signed-off-by: Artur Souza <artursouza.ms@outlook.com> Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

* Update LICENSE to Apache 2.0 Signed-off-by: Artur Souza <artursouza.ms@outlook.com> * Update headers to Apache 2. Signed-off-by: Artur Souza <artursouza.ms@outlook.com> Co-authored-by: Artur Souza <artursouza.ms@outlook.com> Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

Bumps [actions/github-script](https://github.com/actions/github-script) from 1 to 5. - [Release notes](https://github.com/actions/github-script/releases) - [Commits](actions/github-script@v1...v5) --- updated-dependencies: - dependency-name: actions/github-script dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

Issue reference: dapr/docs#2039 Signed-off-by: Will <william.wl.tsai@gmail.com> Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

Bumps [actions/setup-java](https://github.com/actions/setup-java) from 2.4.0 to 2.5.0. - [Release notes](https://github.com/actions/setup-java/releases) - [Commits](actions/setup-java@v2.4.0...v2.5.0) --- updated-dependencies: - dependency-name: actions/setup-java dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Artur Souza <artursouza.ms@outlook.com> Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

* inital draft for config api Signed-off-by: Pravin Pushkar <ppushkar@microsoft.com> * Introducing new client for preview apis and code refactoring Signed-off-by: Pravin Pushkar <ppushkar@microsoft.com> * Unit tests and code refactoring Signed-off-by: Pravin Pushkar <ppushkar@microsoft.com> * Adding integration test Signed-off-by: pravinpushkar <ppushkar@microsoft.com> * Copyright changes Signed-off-by: pravinpushkar <ppushkar@microsoft.com> * Review comments fixes Signed-off-by: pravinpushkar <ppushkar@microsoft.com> * Removed DaprPreviewClientProxy and updated example README Signed-off-by: pravinpushkar <ppushkar@microsoft.com> * Adding validate workflow for cofiguration api example Signed-off-by: pravinpushkar <ppushkar@microsoft.com> * fixing example autovalidation and code coverage Signed-off-by: pravinpushkar <ppushkar@microsoft.com> * Fixing autovalidation and removing getAllConfiguration Signed-off-by: pravinpushkar <ppushkar@microsoft.com> * Fixing review comments Signed-off-by: pravinpushkar <ppushkar@microsoft.com> * Add regex header checkstyle. Signed-off-by: Artur Souza <artursouza.ms@outlook.com> * Fix headers and add javadocs to some. Signed-off-by: Artur Souza <artursouza.ms@outlook.com> Co-authored-by: Artur Souza <artursouza.ms@outlook.com> Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

artursouza

I will give another pass tomorrow. Thanks.

Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

artursouza

Just a minor change.

artursouza · 2022-02-01T23:34:04Z

+   * @return The loopback address String
+   */
+  public static String getHostLoopbackAddress() {
+    return InetAddress.getLoopbackAddress().getHostAddress();


Can this method automatically return the Ipv6 between "[]"?

@artursouza Sorry for the delay, finally had some time to look into this. I changed some unit tests locally to validate IPv6 sidecar IP constants contained within "[]" and it seems like your suggestion is viable - for both URL endpoint invocation formats and Dapr client instantiation. Great recommendation!

However, before I proceed, I need to validate an implementation detail with you. To test for whether an address is IPv6, it would be convenient to leverage the Apache Commons Validator, though you have recommended against bringing in 3P libraries beyond the test scope. Is it possible to get an exception for this case, or do I have to write a custom IPv6 validator within NetworkUtils?

@willtsai Is it complex to write a validator for IPv6 ? both standard pattern and hex compressed pattern along with IPv4 validation?
I am wary of adding a new dependency specifically for one validation ....

Understood - should not be too complex but I wanted to see if the work (and subsequent unit tests) could be avoided. Seems like adding a new dependency is not worth it here, so I'll work on the custom validator.

I started the implementation for this and have a working validator, but then realized that this would introduce inconsistencies into the Property<String> SIDECAR_IP config object since the result would be that the special IPv6 formatting would only get applied when DEFAULT_SIDECAR_IP is used in fallback scenarios, so the IPv6 address would not get "[]" applied to them when they are sourced from the system property name or env variable.

Thus, I think there are two options:

Leave the implementation of adding "[]" to the http address to the client and thus just modifying our test clients accordingly, i.e. code in this PR remain as is.

Make modifications to the Property and/or Properties objects to introduce a new object class IpProperty.java or equivalent that would apply the required IPv6 formatting to the IP address it returns.

@artursouza @mukundansundar - what are your thoughts here? which option would you prefer? (I personally think option 1 introduces less surprises to the client who may not be expecting IPv6 address to be pre-formatted for http URLs)

Signed-off-by: Will <28876888+willtsai@users.noreply.github.com>

cicoyle · 2023-12-21T15:13:57Z

@willtsai mind resolving the conflicts?

Signed-off-by: Will <28876888+willtsai@users.noreply.github.com>

Signed-off-by: Will Tsai <28876888+willtsai@users.noreply.github.com>

willtsai · 2024-01-02T23:34:57Z

@willtsai mind resolving the conflicts?

@cicoyle done, though now i need to investigate the test failures...

Signed-off-by: Will Tsai <28876888+willtsai@users.noreply.github.com>

willtsai · 2024-01-04T22:39:46Z

@cicoyle i've addressed all merge conflicts and test failures, can you please take another look? (i think the build tests just need to be re-run)

Signed-off-by: Will Tsai <28876888+willtsai@users.noreply.github.com>

Signed-off-by: Artur Souza <artursouza.ms@outlook.com>

codecov · 2024-02-19T21:54:26Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (14cc3f8) 77.60% compared to head (d9fd2c0) 77.61%.
Report is 4 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##             master     #649   +/-   ##
=========================================
  Coverage     77.60%   77.61%           
- Complexity     1570     1571    +1     
=========================================
  Files           144      144           
  Lines          4765     4767    +2     
  Branches        554      554           
=========================================
+ Hits           3698     3700    +2     
  Misses          781      781           
  Partials        286      286

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

cicoyle

lgtm - nice to see this coming to a closing 🎉

willtsai · 2024-02-20T17:41:35Z

woo! thanks @cicoyle @artursouza 🥳

willtsai requested review from a team as code owners November 4, 2021 06:09

artursouza suggested changes Nov 12, 2021

View reviewed changes

Comment thread sdk/src/main/java/io/dapr/config/Properties.java Outdated

willtsai changed the title ~~Change "127.0.0.1" to SIDECAR_IP to dynamically account for IPv4 vs. IPv6 environments~~ [WIP] Change "127.0.0.1" to SIDECAR_IP to dynamically account for IPv4 vs. IPv6 environments Nov 13, 2021

mukundansundar added the do-not-merge label Dec 6, 2021

willtsai mentioned this pull request Jan 26, 2022

The sdk should auto-detect ipv4 loopback and ipv6 loopback for daprd communication. #218

Closed

willtsai changed the title ~~[WIP] Change "127.0.0.1" to SIDECAR_IP to dynamically account for IPv4 vs. IPv6 environments~~ Change "127.0.0.1" to SIDECAR_IP to dynamically account for IPv4 vs. IPv6 environments Jan 26, 2022

artursouza and others added 20 commits January 25, 2022 16:28

Generate updated javadocs for 1.3.1 (dapr#643)

5e792b1

Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

Migrated out of JCenter as a Maven repository manager (dapr#648)

09d2bc5

* Upgrade okhttp-mock dependency Version 1.3.2 was never deployed to Maven Central * No longer refer to JCenter, as it has been shut down Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

initial commit: modified ./dapr/client/DaprClientHttpTest.java unit test

0171346

Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

refactored hardcoded 127.0.0.1 localhost IP to use Dapr sidecar IP

d6c18c0

Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

refactored hardcoded 127.0.0.1 localhost IP to use Dapr sidecar IP

4aa8aba

Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

cleaned up unused imports

95f3728

Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

fixed tabbing and imports

cdb5ba8

Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

added utils for getting local host and test utils for ipv6 addresses

943596e

Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

Add auto-assign dapr_bot workflow (dapr#654)

b4874d4

Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

added HTTP client tests for IPv6 addresses

be5b2a9

Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

fixed tests by returning SIDECAR_IP to IPv4 address

f4363d6

Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

changes to address PMD code validation violations

ff7c074

Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

changing back to loopback address for DEFAULT_SIDECAR_IP fallback

aed37ac

Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

Fix vault example + Upgrade Dapr CLI and runtime. (dapr#666)

3ffc34d

* Upgrade Dapr CLI and runtime. Signed-off-by: Artur Souza <artursouza.ms@outlook.com> * Fix expected vault output. Signed-off-by: Artur Souza <artursouza.ms@outlook.com> Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

Add DCO requirements and updated code of conduct (dapr#669)

0504596

Issue reference: dapr/docs#2039 Signed-off-by: Will <william.wl.tsai@gmail.com> Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

mukundansundar requested a review from artursouza January 28, 2022 02:04

artursouza suggested changes Jan 28, 2022

View reviewed changes

Comment thread sdk-actors/pom.xml

Comment thread sdk/pom.xml

updated dependency scope to test

29bf6a6

Signed-off-by: Will Tsai <william.wl.tsai@gmail.com>

artursouza suggested changes Feb 1, 2022

View reviewed changes

willtsai and others added 2 commits July 12, 2022 13:29

Merge branch 'master' into loopback-auto-detect-218

59dca39

Merge branch 'master' into loopback-auto-detect-218

52821d8

Signed-off-by: Will <28876888+willtsai@users.noreply.github.com>

willtsai requested review from artursouza and mukundansundar August 15, 2023 16:37

willtsai added 4 commits January 2, 2024 11:35

Merge branch 'master' into loopback-auto-detect-218

e391e95

Signed-off-by: Will <28876888+willtsai@users.noreply.github.com>

fix NetworkUtils.java

424e70a

Signed-off-by: Will Tsai <28876888+willtsai@users.noreply.github.com>

checkstyle fix in NetworkUtils.java

3797e31

Signed-off-by: Will Tsai <28876888+willtsai@users.noreply.github.com>

fix imports

e8ddb5c

Signed-off-by: Will Tsai <28876888+willtsai@users.noreply.github.com>

willtsai added 3 commits January 4, 2024 13:47

fix ipv6 formatting issues

cbc11cb

Signed-off-by: Will Tsai <28876888+willtsai@users.noreply.github.com>

fix ipv6 formatting issues

904387b

Signed-off-by: Will Tsai <28876888+willtsai@users.noreply.github.com>

fix code style nits

3b39e86

Signed-off-by: Will Tsai <28876888+willtsai@users.noreply.github.com>

willtsai and others added 4 commits January 5, 2024 09:38

refactor formatIpAddress method in actors utls

9c82edb

Signed-off-by: Will Tsai <28876888+willtsai@users.noreply.github.com>

Merge branch 'master' into loopback-auto-detect-218

447edb1

Merge branch 'master' into loopback-auto-detect-218

cbf4f47

Merge branch 'master' into loopback-auto-detect-218

9543fcd

Signed-off-by: Artur Souza <artursouza.ms@outlook.com>

artursouza approved these changes Feb 19, 2024

View reviewed changes

artursouza removed the do-not-merge label Feb 19, 2024

Merge branch 'master' into loopback-auto-detect-218

d9fd2c0

cicoyle approved these changes Feb 19, 2024

View reviewed changes

artursouza approved these changes Feb 20, 2024

View reviewed changes

artursouza merged commit 57b86c5 into dapr:master Feb 20, 2024

Uh oh!

Conversation

willtsai commented Nov 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issue reference

Checklist

Uh oh!

mukundansundar commented Nov 12, 2021

Uh oh!

willtsai commented Nov 12, 2021

Uh oh!

Uh oh!

willtsai commented Dec 6, 2021

Uh oh!

artursouza left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

artursouza left a comment

Choose a reason for hiding this comment

Uh oh!

artursouza Feb 1, 2022

Choose a reason for hiding this comment

Uh oh!

willtsai Mar 14, 2022

Choose a reason for hiding this comment

Uh oh!

mukundansundar Mar 15, 2022

Choose a reason for hiding this comment

Uh oh!

willtsai Mar 16, 2022

Choose a reason for hiding this comment

Uh oh!

willtsai Jul 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cicoyle commented Dec 21, 2023

Uh oh!

willtsai commented Jan 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

willtsai commented Jan 4, 2024

Uh oh!

codecov Bot commented Feb 19, 2024

Codecov Report

Uh oh!

cicoyle left a comment

Choose a reason for hiding this comment

Uh oh!

willtsai commented Feb 20, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

willtsai commented Nov 4, 2021 •

edited

Loading

willtsai Jul 12, 2022 •

edited

Loading

willtsai commented Jan 2, 2024 •

edited

Loading